[CI] Revise the Windows skip logic of UT #2383
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR revises the Windows skip logic for unit tests by improving how skip lists are handled and fixing file patterns. The changes ensure empty lists are properly handled as None values and correct the file skip pattern format.
Key Changes:
- Updated skip list validation to check for both truthiness and non-empty lists
- Fixed the skip pattern from ".py::" to ".py" for entire file skipping
- Added handling for None and tuple values in skip dictionary merging
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| test/xpu/xpu_test_utils.py | Added length checks to skip_list and exe_list validation to ensure empty lists are treated properly |
| test/xpu/windows_skip_cases.py | Corrected dictionary key from "test_decomp" to "test_decomp.py" and updated skip pattern format |
| test/xpu/run_test_with_skip.py | Fixed import statement, updated file skip pattern detection, added handling for None/tuple values in skip dict, and added empty list normalization |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if skip_dict[key] is None: | ||
| merged_skip_dict[key] = [] | ||
| elif isinstance(skip_dict[key], tuple): | ||
| merged_skip_dict[key] = list(skip_dict[key]) | ||
| else: | ||
| merged_skip_dict[key] = skip_dict[key].copy() if skip_dict[key] else [] |
There was a problem hiding this comment.
[nitpick] The nested if-elif-else logic can be simplified. When skip_dict[key] is None or falsy, all branches result in an empty list. Consider consolidating: merged_skip_dict[key] = list(skip_dict[key]) if isinstance(skip_dict[key], tuple) else (skip_dict[key].copy() if skip_dict[key] else [])
| if skip_dict[key] is None: | |
| merged_skip_dict[key] = [] | |
| elif isinstance(skip_dict[key], tuple): | |
| merged_skip_dict[key] = list(skip_dict[key]) | |
| else: | |
| merged_skip_dict[key] = skip_dict[key].copy() if skip_dict[key] else [] | |
| merged_skip_dict[key] = list(skip_dict[key]) if isinstance(skip_dict[key], tuple) else (skip_dict[key].copy() if skip_dict[key] else []) |
chuanqi129
changed the title
| # Windows: Skip entire files using *.py:: pattern | ||
| "test_decomp": [ | ||
| "test_decomp.py::", # Skip entire file on Windows | ||
| "test_decomp.py": [ |
There was a problem hiding this comment.
to align with the naming style you could change test_decomp.py to test_decomp_xpu.py.
| if not skip_list: | ||
| return False | ||
| return any(item.endswith(".py::") for item in skip_list) | ||
| return any(item.endswith(".py") for item in skip_list) |
There was a problem hiding this comment.
This update will make all the test files return True, is it correct?
disable_e2e
disable_distribute